-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add missing null-loader dependency #152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be in dependencies of packages/vue-cli-plugin-vuetify
@KaelWD I did a local build and didn't see it complain, so just to make sure I follow are you referring to the following block? vue-cli-plugins/packages/vue-cli-plugin-vuetify/package.json Lines 34 to 37 in addc5bc
|
Great job @JamesMcMahon tracking this down and fixing it. I'm not sure why @KaelWD wants It would be great to get this merged. |
This PR adds
yes that is the correct location, but I think it should be under |
Yeah I can make those changes. The way you phrased it makes sense @DRoet. Thanks for clarifying. |
7c4aff2
to
31d3724
Compare
Fix issue in vuetifyjs#101 On the fresh project adding the `vue add vuetify` plugin results in a missing dependency: ``` WEBPACK Failed to compile with 9 error(s) Error in ./node_modules/vuetify/lib/components/VImg/VImg.js Module not found: 'null-loader' in '/Users/jamie/temp/vuetify-create-test' ```
31d3724
to
41ea4de
Compare
PR has been updated. I removed the dependency from ad-hoc and added to the package.json. |
Confirmed and tested locally that this works. Note for myself in case I need to make further changes, https://cli.vuejs.org/dev-guide/plugin-dev.html#installing-plugin-locally. |
I am just checking in on this. I see two approvals, is there anything I need to do before this is merged? |
nope, it just needs to get merged/released by someone who has the time to do it |
@johnleider Do you have some time to merge this in and do a release? |
Need to change the loader to use
sassRule.use('null-loader').loader(require.resolve('null-loader'))
scssRule.use('null-loader').loader(require.resolve('null-loader'))
|
Want to create a separate PR for that? I want to scope this PR down to just
installing the dependency.
This has been broken for 6 or so months now and this PR has been sitting
approved for two months so I would like to see it merged so we can build
additional needed changes on top of it.
…On Sun, Apr 26, 2020, 05:48 Kristoffer K. ***@***.***> wrote:
Need to change the loader to use require.resolve('null-loader') otherwise
we're depending on hoisting which is a terrible idea
https://github.com/vuetifyjs/vue-cli-plugins/blob/908f65d35478da2cd9df3569c2ce4c5ca1910468/packages/vue-cli-plugin-vuetify/index.js#L48
Should be
sassRule.use('null-loader').loader(require.resolve('null-loader'))
https://github.com/vuetifyjs/vue-cli-plugins/blob/908f65d35478da2cd9df3569c2ce4c5ca1910468/packages/vue-cli-plugin-vuetify/index.js#L52
Should be
scssRule.use('null-loader').loader(require.resolve('null-loader'))
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG7ZRCHEM7YKSC2W4LV2CDROQGQXANCNFSM4KIQUB4A>
.
|
@KaelWD can we get this merged? :) |
@johnleider friendly ping |
Fix issue in #101
On the fresh project adding the
vue add vuetify
plugin results in a missing dependency: